Skip to content

Conversation

@philip-nikolov-jpmc
Copy link

Basic setup for YAML-based configuration of micro frontends.
Includes tests and readme.
The config will be use in subsequent PRs

@github-actions github-actions bot added product PR or Issue related to the DataHub UI/UX community-contribution PR or Issue raised by member(s) of DataHub Community labels Oct 21, 2025
@codecov
Copy link

codecov bot commented Oct 21, 2025

Bundle Report

Bundle size has no change ✅

@datahub-cyborg datahub-cyborg bot added the needs-review Label for PRs that need review from a maintainer. label Oct 21, 2025
@philip-nikolov-jpmc philip-nikolov-jpmc marked this pull request as draft October 21, 2025 18:42
@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

'module',
'flags',
'permissions',
'navIcon',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we want to keep navIcon required?

*/
export function loadMFEConfigFromYAML(yamlString: string): MFESchema {
try {
console.log('[MFE Loader] Raw YAML:', yamlString);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this may be unnecessary and also can lead to exposure of unnecessary data. Also could be wrapped around only for dev environments if needed.

}
const config = loadMFEConfigFromYAML(mfeYamlRaw);
if (config) {
console.log('[MFE Loader] useMFEConfigFromYAML loaded:', config);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, we don't need to log this on every page

throw new Error('[MFE Loader] Invalid YAML: missing microFrontends array');
}
// Validate each entry, keeping both valid and invalid ones
parsed.microFrontends = parsed.microFrontends.map(validateMFEConfig);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could also just filter out here instead of storing the InvalidMFEConfig and passing around.

Copy link
Contributor

@ani-malgari ani-malgari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also may be i'm lacking context, how do you envision to provide the mfe config since the app will be bundled?

}

export function useDynamicRoutes(): JSX.Element[] {
const mfeConfig = useMFEConfigFromYAML();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the idea here that this will be replaced one day with useMFEConfigFromRemote?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PR or Issue raised by member(s) of DataHub Community needs-review Label for PRs that need review from a maintainer. product PR or Issue related to the DataHub UI/UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants